Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement environmental audio using OpenAL EFX #3182

Draft
wants to merge 105 commits into
base: master
Choose a base branch
from

Conversation

Hiradur
Copy link
Contributor

@Hiradur Hiradur commented Sep 25, 2024

This is a WIP PR for implementing reverb support using the OpenAL EFX extensions. To make it easier for me, this PR is currently based on #3180 and #3181.
The basic foundation is already there. If the listener (camera) is underwater, an underwater effect becomes active.
The preset can be changed to ones listed here using the console command set audio_listener_efx_preset(see #3182 (comment)). Resetting the preset is currently not possible via console.

There is still some work to do:

  • efficiently handle the creation of alEffects
  • make changing the reverb scriptable
  • avoid harsh transitions when presets change
  • automatically activate an in-cockpit preset when appropriate? -> not easy to know whether a truck has a closed cabin or not
    • do not enable it if the listener is very close but outside of the truck (happens because there is a check whether the listener is inside the BB of the driven vehicle, but certain vehicles provide outside camera views)
  • allow to define environmental areas in terrn2 files? -> possible using collision boxes
  • ensure that sound sources always get updated: Sometimes they don't receive updates to e.g. the obstruction filter. This is a bug.
  • Delete alEffects (and potentially alFilters) when returning to main menu.

gcc requires this due to how the struct is initialized in AddonPartFileFormat.cpp
OpenAL 1.1 changed the way how the doppler effect is handled from OpenAL 1.0. Using the old way did not break the build as demanded by the spec but disabled the effect for at least the OpenAL Soft implementation.
update variable names to follow coding style and prefer local variables over method calls
this also allows to disable the effect by setting the doppler factor to 0
This design has less coupling and allows for setting more environmental properties of the listener in the future
@ohlidalp
Copy link
Member

Looks fantastic so far. I'll test on friday.

This is considered WIP since the parameters are configurable, but they get currently overwritten at each frame dependent on the environment of the listener
@Hiradur
Copy link
Contributor Author

Hiradur commented Sep 26, 2024

I'd like to have the reverb presets as well as the air absorption related parameter to be scriptable, manually overridable or to automatically change depending on whether the listener is underwater or not (and perhaps they are in a cockpit or not).
Updating the air absorption parameters of every source every frame even if they haven't changed also seems wasteful.

I haven't decided how to bring all this together yet. Might be necessary to implement a state machine to track where the parameters are allowed to be set from.

This should be considered an incomplete implementation. The line of sight checks/collision detection need to be improved. It does not detect trucks and some meshes on the terrain while it sometimes detects invisible walls
@Hiradur
Copy link
Contributor Author

Hiradur commented Sep 26, 2024

Since somewhere in the OpenAL docs it was suggested to implement occlusion and obstruction effects alongside reverb effects to put more emphasize on the latter, I added a basic obstruction filter.
It's disabled by default and hidden behind the CVar audio_enable_obstruction. The collision detection definitely needs to be improved before this can be enabled by default (see commit message and comments in code).

@ohlidalp
Copy link
Member

ohlidalp commented Sep 26, 2024

I appreciate your focus on scripting, some notes to hopefully make life easier for you:

Improving sounds including terrain/object interference has been on my mind for years, but there's always too much other stuff, regarding scripting try in console 'loadscript demo_script.as', it showcases the current basic audio bindings. There's plenty more examples in repo under /resources/scripts.

Also come hang around other Devs in Discord #dev https://discord.com/channels/136544456244461568/189904947649708032

The AL_EFFECT_EAXREVERB engine is a superset of the AL_EFFECT_REVERB engine. Not all OpenAL implementations might support it. At least the Creative AL driver and OpenAL Soft support this reverb engine and there is a noticable difference in some of the reverb presets. Some of the features it provides might be useful for a more realistic audio experience as well.
This allows the user to set environmental audio parameters from console or scripts without fearing they get overwritten by the engine
This is necessary to change the AIR_ABSORPTION_GAIN_HF property once a preset has been made active. It will also be necessary for panning of reflection and late reverb.
@ohlidalp
Copy link
Member

I'll create a pull request with the new terrain editor during the coming week, then we can work the reverb and obstruction feats into an editor UI, which will both showcase and document it visually.

@Hiradur
Copy link
Contributor Author

Hiradur commented Nov 24, 2024

ODEFs can omit mesh if you set mesh name to "none"

Neat trick, I didn't notice this. Thanks for the info, this makes using collision boxes much more viable.

I'll create a pull request with the new terrain editor during the coming week, then we can work the reverb and obstruction feats into an editor UI, which will both showcase and document it visually.

Sounds great.
As far as the obstruction filter is concerned, it checks against obstruction by terrain, collision boxes, collision meshes, and AABBs of actors. Unlike reverb environments, it doesn't need special entities; hence there wouldn't be a need for UI elements in the terrain editor.
In theory, realism could be improved by making the obstruction filter parameters dependent on acoustic material properties and thickness. I don't see this within the scope of this PR though since it should be done with diffraction path simulation for more realistic behavior. Many older games just use line of sight checks and apply a simple obstruction filter if an obstacle was detected, which is also what is done in this PR at the moment. Currently there is only one filter parameter set for every obstruction scenario. One could create more for different scenarios (e.g. there is probably a difference if a sound is obstructed by a hill, a house or a car). For the most realistic behavior, however, the parameters should be determined dynamically based on the diffraction path properties.

… actor

The implementation did not differentiate whether the actor had an open or closed cabin. For instace, it incorrectly applied the obstruction effect when the listener was in actors with open cabins, e.g. buggies. There does not seem to be an easy way to determine whether an actor has an open or closed cabin. An expensive(?) alternative might be to check against collisions with the mesh of an actor.
The console command was largely redundant to working with the audio_doppler_factor CVar. Instead of the previous way of triggering updates of the doppler factor via the message queue, the doppler factor is now updated each frame from the audio_doppler_factor CVar.
cryham added a commit to stuntrally/stuntrally3 that referenced this pull request Nov 26, 2024
@cryham
Copy link
Contributor

cryham commented Nov 26, 2024

Hi.
I have integrated this code into SR3 now, on branch sound 🎉. It's still WIP though, I got some stuff commented out and I just started restoring SR3 sound using it.

I have a few comments, about this PR's code style. Obviously, I'm not a RoR developer, so it's up to you.

  • inconsistencies with if( vs if ( (I'd say with space is better), also for( vs for (
  • some variables start with m_ and the rest don't (I'd say m_ isn't needed)
  • some code is densely packed, few empty lines would be better, e.g. under // OpenAL EFX stuff
  • less readable return statements in same line e.g. bool isDisabled() { return disabled; },
    few methods are very packed (getSoundName, getSoundPitch, getSound).
    I'd either move all to next line (unless for short get/set), or put 2 spaces before { and } (in SR3 code)
  • some very long names e.g. m_listener_is_inside_the_player_coupled_actor
  • generally some methods are big, I usually put 2 empty lines at start and end (in SR3 code)

I fixed all those and more in this stuntrally/stuntrally3@3588cad
but surely many things there are related to SR3 code only. E.g. I don't have CVars, scripts etc.

@Hiradur
Copy link
Contributor Author

Hiradur commented Nov 27, 2024

Great to see it being tried out in SR3 already and I appreciate your feedback. I still intend to do some cleanup and refactoring before gettings this merged.

Onto your suggestions:

inconsistencies with if( vs if ( (I'd say with space is better), also for( vs for (

I should fix that.

some variables start with m_ and the rest don't (I'd say m_ isn't needed)

All code I added should follow the RoR coding style guidelines, which say //STYLE: privates are prefixed m_. Since a lot of the audio code predates my work, there is a mix of styles, unfortunately. This problem also exists elsewhere in the RoR codebase.

some code is densely packed, few empty lines would be better, e.g. under // OpenAL EFX stuff

I'll take a look if it can be improved. I'll probably just copy your improvement.

less readable return statements in same line e.g. bool isDisabled() { return disabled; }

The coding style guidelines have an example of this, which I followed since I like it for simple getters:

int  GetMemberValue() { return m_private_member_variable; }  //STYLE: Getter function -> `Get` in name

few methods are very packed (getSoundName, getSoundPitch, getSound).
I'd either move all to next line (unless for short get/set), or put 2 spaces before { and } (in SR3 code)

The mentioned functions aren't part of this PR and cleaning them up should be left for another PR.

some very long names e.g. m_listener_is_inside_the_player_coupled_actor

I prefer long variable/function names over adding comments to prevent comments getting outdated and make the code read more like natural language (see also clean code principles). I could change it, if desired.
I eliminated this particular variable in the last commits since it was used for enabling the obstruction filter when the listener was inside a truck/actor. However, since RoR also has trucks with open cabins, the obstruction filter shouldn't be enabled unconditionally. I saw you renamed it to listener_inside_player. If you want to keep the variable I recommend renaming it since it's really about whether the listener is inside a truck/actor.

@cryham
Copy link
Contributor

cryham commented Nov 27, 2024

Cool. I am not a developer here, just went through the code BTW. Definitely ohlidalp has the say, and I forgot there is coding style guidelines, so better to keep to it.

Anyways I'm not sure I will use this code, it seems buggy right now, not sure why.
I may keep my previous Sound code fork of RoR, which I modified and it works fine (I added reverb, it has ogg, 2D sounds (for Hud and for all in Split Screen), from config file I also removed triggers etc).
I also don't see in any of them, the 2 features which I'd like most now: dynamic sounds and ambient sound (ogg).
I saw in this video that e.g. hit sounds (for hitting barrels, boxes in SR3) would need to be deleted/removed after playing and there also should be a pool for few at most.
I don't see anything like this in RoR, remove is only done after deleting actor, and on load all needed sources are created, no dynamic sounds on play. This isn't an issue for RoR just for SR3. But yeah I wrote that since you probably know (RoR's) sound code best now.

@Hiradur
Copy link
Contributor Author

Hiradur commented Nov 27, 2024

Anyways I'm not sure I will use this code, it seems buggy right now, not sure why.

Can you elaborate? I didn't notice any bugs in RoR.

I also don't see in any of them, the 2 features which I'd like most now: dynamic sounds and ambient sound (ogg). I saw in this video that e.g. hit sounds (for hitting barrels, boxes in SR3) would need to be deleted/removed after playing and there also should be a pool for few at most.

I see buffer management as a different topic. For this PR I only want to focus on environmental audio implemented via OpenAL EFX. I didn't pay too much attention to the already existing buffer management but I think your assessment is correct and that there is room for improvement, especially for ambient sounds.

@cryham
Copy link
Contributor

cryham commented Nov 27, 2024

No I meant it's buggy in SR3, after my somewhat reworked version. And different code in SR3 compared to RoR. I don't have such setup as in RoR with all SoundTriggers, ModulationSources in ActorSpawner.cpp, I don't have that and I just set gain and pitch myself in update. Could be something bad with this in SR3.

…Manager class and rename it to UpdateListenerEnvironment
While the current implementation could be improved in terms of filter parameters and line of sight checks against actors, it is solid enough to no longer be considered experimental.
…tor's unit

This behavior more closely resembles that of Ogre's intersects methods.
Return the default efx preset not just for the listener position but for any position in the game that has no other efx preset set
On most maps, obstruction by terrain might be the least likely option; for optimization purposes it is checked for last
While this fixes driving over the ramp on nHelens modded, there are still some cases where the obstruction filter is unstable, e.g. when driving with some vehicles on the roads which use collision boxes on nHelens modded
For the sake of performance, it might make sense to cache which nodes are attached to exhausts/turboprops/-jets in the future.
There is no simple way to tell whether a truck has a closed cabin or not. This option exists to force the obstruction filter inside an actor if desired.

Since the check if the listener is inside the player's coupled actor is based on the actor's AAB, the obstruction filter might get enabled if the listener is close to but not inside the actor.

Future work: it might be better to perform line of sight checks against the mesh of a vehicle, which could solve both problems.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants